Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Bootstrap v4 alpha 4 in v1 #170

Merged
merged 5 commits into from
Sep 25, 2016
Merged

Support Bootstrap v4 alpha 4 in v1 #170

merged 5 commits into from
Sep 25, 2016

Conversation

justin808
Copy link
Member

@justin808 justin808 commented Sep 10, 2016

This change is Reviewable

@justin808
Copy link
Member Author

@rmobis @Andre-Gl can you please verify this works with webpack v1? Thanks.

@justin808
Copy link
Member Author

@Judahmeek, could you maybe try this out?

@Judahmeek
Copy link
Contributor

Yes, I'll try it out.

@@ -12,7 +12,7 @@ export default function(module, bootstrapVersion, bootstrapPath) {
const scriptsPath = (
parseInt(bootstrapVersion, 10) === 3 ?
['assets', 'javascripts', 'bootstrap'] :
['dist', 'js', 'umd']
['js', 'dist']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add new path also to readme jquery module loader.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this readme jquery module loader you speak of?

@outdooricon
Copy link

We're really looking forward to seeing this in. Alpha2 is broken in multiple ways for us that alpha4 fixes

@justin808
Copy link
Member Author

@Judahmeek @outdooricon I can't finish this for at least a week. Can one of you guys submit a new PR with this, fixing the things that are broken.

thanks!

@Judahmeek
Copy link
Contributor

@justin808 I'll give it a shot.

@Judahmeek
Copy link
Contributor

Judahmeek commented Sep 15, 2016

@justin808 All the errors were due to the fact that the examples have their own modules of bootstrap-loader instead of being linked to local code. Is there a particular reason for that or can I attempt to create a local dependency with symlinks to the local source code?

Also, the examples' webpack.configs need to be updated on line 56/57 (just removing the /umd\).

@justin808
Copy link
Member Author

@Judahmeek

https://github.com/shakacode/bootstrap-loader/tree/v1/examples/basic

See the line

npm run install-local

We can hard code the values in the file, but they need to be changed every release.

Maybe we just need to change the docs? on this page.

@Judahmeek
Copy link
Contributor

Judahmeek commented Sep 15, 2016

We can probably just put this down to my inexperience as a contributor. I overlooked the README, went straight to package.json, and overlooked npm install-local there because I wasn't considering the need for the examples to be used as integration tests (and thus the need for some form of linking).

@igama
Copy link

igama commented Sep 23, 2016

Hey, Any update on this ? :)

@justin808 justin808 merged commit 142c838 into v1 Sep 25, 2016
@justin808 justin808 deleted the support-alpha-4-in-v1 branch September 25, 2016 06:21
@justin808
Copy link
Member Author

I just merged this to the V1 branch as is. There might be a couple more changes and then a release for v1.


Review status: 0 of 9 files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@justin808
Copy link
Member Author

Review status: 0 of 9 files reviewed at latest revision, 2 unresolved discussions.


src/utils/createBootstrapRequire.js, line 15 at r2 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

What is this readme jquery module loader you speak of?

@Judahmeek

I'll update this.

module: {
  loaders: [
    // Use one of these to serve jQuery for Bootstrap scripts:

    // Bootstrap 3
    { test:/bootstrap-sass[\/\\]assets[\/\\]javascripts[\/\\]/, loader: 'imports?jQuery=jquery' },

    // Bootstrap 4
    { test: /bootstrap[\/\\]dist[\/\\]js[\/\\]umd[\/\\]/, loader: 'imports?jQuery=jquery' },
  ],
},


Comments from Reviewable

@justin808
Copy link
Member Author

@outdooricon @frickt @igama @rmobis @Andre-Gl:

Released 1.2 with support for alpha.v4!

@outdooricon
Copy link

@justin808 sweet, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants